Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Oct 8, 2025

Issue being fixed or feature implemented

hana still uses spaces before literal operator, while newer clang enforces C++23 requirement

In file included from ./evo/dmnstate.h:19:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/for_each.hpp:15:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/concept/foldable.hpp:19:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/fold_left.hpp:20:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/unpack.hpp:16:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/at.hpp:16:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/concept/iterable.hpp:20:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/drop_front.hpp:20:
In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/integral_constant.hpp:13:
DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/bool.hpp:198:35: error: identifier '_c' preceded by whitespace in a literal operator declaration is deprecated [-Werror,-Wdeprecated-literal-operator]
  198 |         constexpr auto operator"" _c() {
      |                        ~~~~~~~~~~~^~
      |                        operator""_c

What was done?

Adds -Wno-error=deprecated-literal-operator for clang

How Has This Been Tested?

Build locally successfully

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

@knst knst added this to the 23 milestone Oct 8, 2025
@github-actions
Copy link

github-actions bot commented Oct 8, 2025

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

The change updates configure.ac to add a compile-time check and suppression for the deprecated-literal-operator warning when -Werror is enabled. It introduces an AX_CHECK_COMPILE_FLAG for -Wdeprecated-literal-operator that, if supported, appends -Wno-error=deprecated-literal-operator to NOWARN_CXXFLAGS. The check is guarded by an AC_LANG_SOURCE test to skip non-GCC compilers, is executed only within the enable_werror branch, and is placed before the existing -Wattributes suppression.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately summarizes the primary change by indicating it fixes a compilation error with clang-20 without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed The pull request description directly addresses the compilation failure with newer clang due to Boost.Hana’s deprecated literal operator syntax and explains the added compiler flag to suppress this error. It clearly relates to the changes in configure.ac and includes context on testing and checklist status. The description provides sufficient detail about the problem, the solution, and verification steps, making it relevant and on-topic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb01c5a and 813f153.

📒 Files selected for processing (1)
  • configure.ac (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: win64-build / Build source

`hana` still uses spaces before literal operator, while clang enforces C++23 requirement
    In file included from ./evo/dmnstate.h:19:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/for_each.hpp:15:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/concept/foldable.hpp:19:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/fold_left.hpp:20:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/unpack.hpp:16:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/at.hpp:16:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/concept/iterable.hpp:20:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/drop_front.hpp:20:
    In file included from DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/integral_constant.hpp:13:
    DASH/depends/x86_64-pc-linux-gnu/include/boost/hana/bool.hpp:198:35: error: identifier '_c' preceded by whitespace in a literal operator declaration is deprecated [-Werror,-Wdeprecated-literal-operator]
      198 |         constexpr auto operator"" _c() {
          |                        ~~~~~~~~~~~^~
          |                        operator""_c
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
configure.ac (1)

459-460: Consider improving the error message for clarity.

The error message states "Non-clang compiler detected, not setting flag", but the guard condition also explicitly excludes the Intel compiler (even if it's clang-based). While the current message addresses the main concern from the previous review, consider either:

  1. Capitalizing "Clang" to match the style used in comments above (line 456), or
  2. Making the message more explicit about the Intel compiler exclusion: "Not compiling with Clang (or using Intel compiler), not setting flag"

This would align with the suggestion from the previous review and make the exclusion criteria completely clear. That said, the current pattern is consistent with other similar checks in this file (e.g., lines 469, 479, 490), which only mention the primary compiler being targeted.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 813f153 and b396f98.

📒 Files selected for processing (1)
  • configure.ac (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: mac-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: win64-build / Build source
🔇 Additional comments (1)
configure.ac (1)

456-463: Logic and placement are correct.

The implementation correctly:

  • Places the check within the enable_werror branch (appropriate for a -Wno-error=... flag)
  • Uses the preprocessor guard to restrict the flag to Clang (excluding Intel compiler)
  • Adds the flag to NOWARN_CXXFLAGS to prevent the deprecation warning from being treated as an error

This effectively addresses the compilation failure with clang-20 and Boost.Hana as described in the PR objectives.

fi
ERROR_CXXFLAGS=$CXXFLAG_WERROR

dnl -Wdeprecated-literal-operator cause problems with clang 20. Do not treat these warnings as errors.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix grammatical error in comment.

The comment uses "cause problems" but should use "causes problems" (singular verb to match the singular subject).

Apply this diff:

-  dnl -Wdeprecated-literal-operator cause problems with clang 20. Do not treat these warnings as errors.
+  dnl -Wdeprecated-literal-operator causes problems with clang 20. Do not treat these warnings as errors.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dnl -Wdeprecated-literal-operator cause problems with clang 20. Do not treat these warnings as errors.
dnl -Wdeprecated-literal-operator causes problems with clang 20. Do not treat these warnings as errors.
🤖 Prompt for AI Agents
In configure.ac around line 456, the comment has a grammatical error: change
"cause problems" to "causes problems" so the singular subject matches the
singular verb; update the line to read "dnl -Wdeprecated-literal-operator causes
problems with clang 20. Do not treat these warnings as errors."

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK b396f98

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK b396f98

@PastaPastaPasta PastaPastaPasta merged commit 6de66a4 into dashpay:develop Oct 13, 2025
34 of 36 checks passed
@knst knst deleted the fix-clang-hana branch October 13, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants